Write ignored lines to cache meta#20747
Conversation
This comment has been minimized.
This comment has been minimized.
|
Ping on this one. |
|
Did you try to look at the physical cache size before&after? Is there any significant impact? (mypy would be a bad benchmark as there are almost no ignores here, but some medium repo from primer should do) Would it make sense to restrict this cache write to imports - not error codes, but comments attached to |
|
It is like extra ~10 bytes per type ignore. Typical cache meta size is like 1kB, so this becomes visible if you have more than a dozen ignores per file.
Hm, actually if it is easy to do, I will try to filter (using |
| write_int_bare(data, len(self.ignored_lines)) | ||
| for line, codes in self.ignored_lines.items(): | ||
| write_int(data, line) | ||
| write_str_list(data, codes) |
There was a problem hiding this comment.
If this becomes a bottleneck, we could use an integer id to represent some of most common error codes.
There was a problem hiding this comment.
Also we could potentially filter common error codes that we know won't be generated for import related errors, but this is a bit dangerous, as we'd need to ensure these ignored lines are used for cases where they work. We could rename the variable though to reduce confusion.
In any case, maye this won't be an issue -- we should probably only optimized if this appears to have some impact in real-world projects.
There was a problem hiding this comment.
Yeah, I will probably just do some easy optimization(s) for now (and will probably rename it to something like import_ignore_lines).
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
3 participants
This fixes a bug discovered while working on #20742, after all it turns out this is not related to #20105 (which is caused by a bug when using
--no-namespace-packageswith namespace packages).This is a bit unfortunate, since cache metas are already quite bulky, but I don't see another way, since we emit import errors during graph loading, before the caller/importer is either parsed or deserialized. So we don't know if there are any type ignores in the caller.
Note only the first test I added was broken, the second is just a mirror image of the first test I add just in case.